Skip to content

DEV-1090: feat(login): migrate to unified auth flow#237

Open
hcarneiro wants to merge 8 commits into
projects/DEV-908-unified-loginfrom
feat/DEV-908-unified-login
Open

DEV-1090: feat(login): migrate to unified auth flow#237
hcarneiro wants to merge 8 commits into
projects/DEV-908-unified-loginfrom
feat/DEV-908-unified-login

Conversation

@hcarneiro

@hcarneiro hcarneiro commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrates the Fliplet CLI login to the unified auth flow and adds the
Fliplet.Auth JS API reference to the developer docs.

CLI login (fliplet-login.js, lib/organizations.js)

  • Replace the removed /v1/auth/third-party endpoint with the unified
    /v1/auth/login?return=callback&callback=http://localhost:9001/callback contract
  • Parse ?token=<token> (new contract) instead of ?auth_token=<token>, via url.parse
  • Route the local callback server on the parsed pathname (/login, /success,
    /callback) instead of a loose req.url.match(/.../). The unified callback now
    carries the user's data (&user=<url-encoded-json>), so a substring match would
    misroute any user whose email or name contains "login"/"success" — breaking their
    sign-in. Pathname matching is exact.
  • Multi-org users: pick the first org by default instead of hard-failing, and print
    a note on how to switch (fliplet organization <id>)
  • All error paths write an HTTP response + process.exit(1) (prevents browser
    retries that triggered duplicate errors)
  • Read auth_token lazily in organizations.js (the module-load-time read was stale
    on first login, since sign-in happens after module init)

Docs (docs/API/fliplet-auth.md + sidebar)

  • New Fliplet.Auth JS API reference (signIn, currentUser, isSignedIn,
    getToken, signOut, onChange), verified against the shipped
    public/assets/fliplet-auth/1.0/auth.js on fliplet-api master
  • Security: the getToken() example sends the auth token to the Fliplet API only,
    with a warning to never send it to third-party endpoints (treat it like a password)
  • onChange documented as firing only for Fliplet.Auth.signIn()/signOut()
  • currentUser() shape includes region and legacy

Test plan

  • Run node fliplet-login.js (or fliplet login if linked) — browser opens to the unified sign-in page
  • Sign in with email/password — CLI prints success and org info
  • Sign in with SSO (Google/Microsoft) — same flow works
  • Multi-org user — first org picked, switch note printed
  • User whose email/name contains "login" or "success" (e.g. john.login@acme.com) — sign-in completes (regression test for the routing fix)
  • Close the browser without completing — CLI exits cleanly
  • Docs render correctly on the developer-docs site (callouts, code blocks)

Note on sequencing

The Fliplet.Auth "signed into your app" experience depends on fliplet-api PR #8199
(restores the flipletLogin passport for embedded/return-mode logins). Publish these
docs after #8199 lands so the cross-widget "signed in" behaviour they describe is true
end-to-end.

🤖 Generated with Claude Code

Replaces the legacy /v1/auth/third-party endpoint (which is being
removed from the API as part of DEV-908) with the same unified sign-in
URL the Fliplet Login widget and VS Code extension now use. The
allow-list on the API already includes http://localhost:9001/callback
so no server-side change is needed for the CLI path.

Contract change: the callback URL now receives ?token=<token>&user=...
instead of ?auth_token=<token>. Parse the token with url.parse(...).query
so we handle any additional params the API may append.

Drive-by fixes to the callback handler surfaced while testing:

- Multi-org users no longer hard-fail. The previous code rejected any
  user belonging to more than one organization with "You belong to
  multiple organizations." Now we pick the first organization by
  default and print a note telling the user how to switch afterwards
  via `fliplet list-organizations` + `fliplet organization <id>`.
- Every error path now writes an HTTP response to the browser AND
  calls process.exit(1). The old code left connections hanging, which
  caused the browser to retry and fire the callback handler twice —
  leading to the "multiple organizations" error printing twice.

package-lock.json: drift from 7.0.0 to 7.0.1 (matches package.json,
unrelated to this change).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hcarneiro hcarneiro changed the title feat(login): migrate to unified auth flow DEV-1037: feat(login): migrate to unified auth flow Apr 10, 2026
@hcarneiro hcarneiro requested review from Arpanexe and tonytlwu April 10, 2026 09:12
hcarneiro and others added 4 commits April 14, 2026 19:37
The organizations module captured auth_token at require-time. When
`fliplet login` runs as a single process — sign-in happens via a
browser callback AFTER the module was loaded — the captured token
was whatever was in configstore at startup (null on first login),
and the subsequent getOrganizationsList() call failed with "You must
login first with: fliplet login".

Moving the user/auth_token lookup inside the function makes it read
the up-to-date configstore value every call.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New developer documentation for the Fliplet.Auth SDK (fliplet-auth
package). Covers:

- signIn / signOut / currentUser / isSignedIn / getToken / onChange
- Common patterns (gated screens, profile display, sign-out button,
  authenticated API requests, reacting to auth state changes)
- What Fliplet.Auth does NOT do (no user creation from apps, no
  app-specific user data management)

Also registers the doc in the API-Documentation.md index under the
alphabetical "Auth" entry.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review pass against references/style-guide.md and references/dev-docs-rules.md:

- Add YAML frontmatter with description field (required for dev docs)
- Remove all horizontal rules (---) per style guide
- Promote the "can't create users from apps" caveat to a <p class="warning">
  callout in "Before you start" — previously buried at the end
- Disambiguate Fliplet.Auth vs Fliplet.User vs data source login on
  first use rather than only mentioning at the end
- First signIn() example includes .catch() so copy-paste is safe
- Make Error rejection cases explicit with exact message strings
- Remove `region` from the documented currentUser() shape (internal
  detail derived from token prefix, not part of public contract)
- Clarify signIn() vs currentUser() return-shape asymmetry inline at
  the point of discovery rather than in a separate "gotchas" section
- Add "if server-side logout fails" behaviour note under signOut()
- All code examples guard against null token / missing user per
  dev-docs-rules ("Code examples must match documented constraints")

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a link to the Fliplet.Auth reference doc in the JS API →
Libraries section of the sidebar, alphabetically between Core and
Communicate, with the NEW label.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hcarneiro hcarneiro changed the title DEV-1037: feat(login): migrate to unified auth flow DEV-1090: feat(login): migrate to unified auth flow May 1, 2026
… review)

The unified callback URL carries the signed-in user's data in the query
string (`?token=...&user=<url-encoded-json>`). The local callback server
routed with loose `req.url.match(/login|success|callback/)` against the
whole URL, so any user whose email or name contains "login" or "success"
(e.g. john.login@acme.com, anyone named "Success") would match the
earlier /login or /success branch instead of /callback — breaking their
sign-in deterministically (redirect loop, or a silent success page that
exits without ever storing the token).

The old `?auth_token=`-only callback didn't carry user data, so the
migration to the `&user=` payload is what exposed this.

Fix: parse req.url once at the top and switch on the exact `pathname`
(`/login`, `/success`, `/callback`) instead of substring-matching the
full URL. No behaviour change for the happy path; removes the
collision surface entirely.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 15, 2026

Copy link
Copy Markdown

Deploying fliplet-cli with  Cloudflare Pages  Cloudflare Pages

Latest commit: e88bbc4
Status:🚫  Build failed.

View logs

@hcarneiro hcarneiro requested review from zeryabkhan91 and removed request for Arpanexe and tonytlwu June 15, 2026 11:05
… (review)

Review of the Fliplet.Auth docs against the shipped
public/assets/fliplet-auth/1.0/auth.js on fliplet-api master:

- Security: the getToken() example sent the Fliplet Auth-Token to a
  third-party origin (https://api.example.com). That token grants full
  access to the user's Fliplet account — sending it off the Fliplet
  origin is a credential leak. Replaced with a Fliplet.API.request call
  to the Fliplet API and added a warning callout (treat like a password;
  only ever send it to the Fliplet API; use a backend-issued token for
  your own backend). Removed the now-duplicate example.
- Correctness: onChange only fires for Fliplet.Auth.signIn()/signOut()
  (notifyChangeListeners is module-private). The old copy claimed it was
  useful for sign-ins via "a different mechanism (Fliplet Login component
  on another screen)" — it does not fire for those. Reworded as a quote
  callout stating the real scope.
- Correctness: currentUser() also returns `region` and `legacy`
  (readStoredUser, auth.js:80-86); the documented shape listed only
  id/email/userRoleId. Added the two fields.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zeryabkhan91

zeryabkhan91 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Substantial item — token-in-URL contradicts the doc this PR ships

The login path this PR wires up calls auth.setUserForToken(authToken), and lib/auth.js authenticates with the token in the URL query string:

request({ url: config.api_url + 'v1/user?auth_token=' + authToken })

Meanwhile the new docs/API/fliplet-auth.md added in this same PR explicitly warns: "Never put it in the URL, log it, or send it to a third-party endpoint." So the documented guidance and the code path disagree.

It's pre-existing (lib/auth.js isn't in this diff), so not introduced here — but this is the natural PR to fix it, given it's part of the token-transport migration epic and it ships the doc preaching the opposite. Suggested fix: switch setUserForToken to the header transport:

request({
  url: config.api_url + 'v1/user',
  headers: { 'Auth-Token': authToken }
})

Low-risk one-liner that makes the CLI practice what its new doc preaches. (Same ?auth_token= may exist on other lib/auth.js/lib/organizations.js calls — worth a quick grep while in there.)

Also: CI shows Cloudflare Pages 🔴 failing (CircleCI passes). Since this PR adds docs/API/fliplet-auth.md and edits docs/_layouts/default.html, please confirm the red docs-preview check is unrelated infra and not an actual Jekyll/docs-build break before merge.


A few nits (non-blocking — the core migration looks correct):

  • source=CLI is silently dropped. The CLI builds …/v1/auth/login?…&source=CLI, but the unified login page (views/login.pug) sets the auth-loader's source from isEmbedded and does not read req.query.source — so the resulting session gets source: undefined, not 'CLI'. Either the server needs to honour ?source= for embedded consumers, or drop the dead param here. Worth confirming what the CLI session source is meant to be.

  • .listen(9001) hardcodes the port while const port = 9001 and baseUrl already use the const. .listen(port) is tidier (pre-existing, just right next to the change).

  • user callback param is parsed away. The contract delivers ?token=…&user=<url-encoded-json>, but the callback handler uses only token and re-fetches the user via setUserForToken/v1/user. That's fine (fresher data), just noting the extra round-trip — and the inline comment mentions the user payload it doesn't actually consume.

  • url.parse is legacy-deprecated in modern Node, but it's the idiomatic choice for a bare req.url path (WHATWG new URL() needs a base), so acceptable here — flagging only for awareness.

  • Heads-up (cross-PR, not a defect): the CLI is a return=callback consumer, so once the API-side PS-1956 fix (#8199) lands, CLI logins will start receiving a flipletLogin passport. Same epic — worth tracking the two together.

… (review)

Addresses Zeryab's review on #237.

Token transport (the substantial item): the CLI authenticated by putting
the token in the URL query string, while the Fliplet.Auth doc this PR
ships warns "never put it in the URL." Tokens in URLs leak into server /
proxy / CDN access logs and Referer headers. Moved all five call sites to
the `Auth-Token` header — the API reads `req.headers['auth-token']` first
(libs/authenticate.js:518), ahead of the query param, so this is a safe
swap for every endpoint:
  - lib/auth.js setUserForToken()  → v1/user
  - lib/auth.js isAdmin()          → v1/user
  - lib/organizations.js           → v1/organizations (also switched the
                                     string-URL request() form to object
                                     form so it can carry headers)
  - lib/publish.js                 → v1/widgets (POST)
  - lib/template.js compile()      → v1/widgets/compile (POST)

Nits:
- Drop the dead `&source=CLI` query param. The unified login page derives
  the session source from `isEmbedded` (views/login.pug) and never reads
  `?source=`, so it tagged nothing. Dropped on the CLI side rather than
  changing the API.
- `.listen(9001)` → `.listen(port)` (the `port` const already existed).
- Comment fix: callback `user` payload is url-encoded JSON, not base64.

All five files pass `node --check`; no `?auth_token=` remains in any URL.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@zeryabkhan91 zeryabkhan91 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants